Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sysconfig support for sysv #37

Merged
merged 1 commit into from
Oct 23, 2014

Conversation

dblessing
Copy link
Contributor

Source /etc/sysconfig/consul, if present. This will allow users to override PID_FILE or LOG_FILE if they wish. Also, it will allow setting GOMAXPROCS. This is maybe the most important feature this provides because otherwise GOMAXPROCS defaults to 1, which is not recommended. If the user does not set GOMAXPROCS after this change, it will default to 2.

I don't believe managing /etc/sysconfig/consul is necessary within this module. It's not a required file for this to work and users can optionally manage that file in a profile or wrapper module.

solarkennedy added a commit that referenced this pull request Oct 23, 2014
@solarkennedy solarkennedy merged commit 29ab4ed into voxpupuli:master Oct 23, 2014
@solarkennedy
Copy link
Contributor

Yea, I saw we were not managing GOMAXPROCS the other day when I was looking at some upstream init stuff.

I disagree on managing the file. Managing the configuration of consul is definitely within the scope of this module.

@dblessing
Copy link
Contributor Author

Ok, fair enough :) Managing the sysconfig file would be fine. One of the main reasons I didn't is because I wasn't sure how you would handle separating that out since other OS's/init systems won't use that.

@EvanKrall
Copy link
Contributor

This module is already managing the init scripts, so we could ensure all the flavors of init script source /etc/default/consul.

@dblessing
Copy link
Contributor Author

@EvanKrall that actually sounds like a great idea. With that in place we could make all the init scripts flat files and not have the template stuff to worry about - manage all via defaults.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants